Skip to content

fix: bun lockfile detection - #1

Merged
dewabisma merged 2 commits into
mainfrom
beast/fix-bug-detecting-bun-lockfile
Aug 27, 2026
Merged

fix: bun lockfile detection#1
dewabisma merged 2 commits into
mainfrom
beast/fix-bug-detecting-bun-lockfile

Conversation

@dewabisma

Copy link
Copy Markdown
Collaborator

Before it was failing bun.lockb even though we have bun.lock beside it.

@dewabisma
dewabisma requested a review from n13 August 27, 2026 09:57

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

Verdict: REQUEST_CHANGES — the new same-directory exception can silently bypass the dependency cooldown when bun.lockb is a real lockfile rather than an empty host-detection marker.

Blocking finding:

  • packages/dependency-cooldown/src/scan.ts:29: omitBunLockbCoveredByTextLock drops every bun.lockb whenever a sibling bun.lock exists, without checking that the binary file is an empty marker or that the repository's Bun version consumes the text file. Bun only switched its default lockfile from bun.lockb to bun.lock in v1.2, so older pinned environments still use the binary file. I reproduced this with Bun 1.1.33: starting from both files, I changed is-number from 7.0.0 to 6.0.0 in package.json and regenerated only bun.lockb; Bun installed 6.0.0, but this action logged Lockfiles: bun.lock, 0 newly introduced dependency version(s), and exited 0 because the decoy text lock was unchanged. A PR can therefore change the dependency graph actually installed by CI without any cooldown check. At minimum, only omit a verified zero-byte marker (for both head and base), reject any non-empty bun.lockb, document the required Bun version, and add a regression test with divergent non-empty lockfiles. Bun's migration documentation also directs users to delete bun.lockb after generating bun.lock: https://bun.sh/docs/pm/lockfile

Validation:

  • git diff --check f974da1e...173d6177 — passed.
  • npm ci — passed (reported one pre-existing low-severity audit item).
  • npm run typecheck — passed.
  • npm run build plus committed-bundle diff check — passed; bundle is current.
  • npm test — passed, 80/80 tests.
  • Bun 1.1.33 divergent-lockfile reproduction — action incorrectly passed with zero checked versions.

@dewabisma

Copy link
Copy Markdown
Collaborator Author

We will expect to see bun.lockb with bun.lock together because cloudflare will require bun.lockb to detect the project use bun as package manager. But we don't care about bun.lockb, we only care for bun.lock so if there is no bun.lock only bun.lockb we should fail.

@dewabisma

Copy link
Copy Markdown
Collaborator Author

Reviewer model: GPT Sol

Verdict: REQUEST_CHANGES — the new same-directory exception can silently bypass the dependency cooldown when bun.lockb is a real lockfile rather than an empty host-detection marker.

Blocking finding:

  • packages/dependency-cooldown/src/scan.ts:29: omitBunLockbCoveredByTextLock drops every bun.lockb whenever a sibling bun.lock exists, without checking that the binary file is an empty marker or that the repository's Bun version consumes the text file. Bun only switched its default lockfile from bun.lockb to bun.lock in v1.2, so older pinned environments still use the binary file. I reproduced this with Bun 1.1.33: starting from both files, I changed is-number from 7.0.0 to 6.0.0 in package.json and regenerated only bun.lockb; Bun installed 6.0.0, but this action logged Lockfiles: bun.lock, 0 newly introduced dependency version(s), and exited 0 because the decoy text lock was unchanged. A PR can therefore change the dependency graph actually installed by CI without any cooldown check. At minimum, only omit a verified zero-byte marker (for both head and base), reject any non-empty bun.lockb, document the required Bun version, and add a regression test with divergent non-empty lockfiles. Bun's migration documentation also directs users to delete bun.lockb after generating bun.lock: https://bun.sh/docs/pm/lockfile

Validation:

  • git diff --check f974da1e...173d6177 — passed.
  • npm ci — passed (reported one pre-existing low-severity audit item).
  • npm run typecheck — passed.
  • npm run build plus committed-bundle diff check — passed; bundle is current.
  • npm test — passed, 80/80 tests.
  • Bun 1.1.33 divergent-lockfile reproduction — action incorrectly passed with zero checked versions.

So: the issue is not valid. bun.lockb-only fails closed today. Preferring bun.lock when both exist is the intended rule, not a bug. The “reject non-empty bun.lockb next to bun.lock” ask is extra hardening, not something current behavior gets wrong.

@dewabisma
dewabisma requested a review from n13 August 27, 2026 10:52

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

Verdict: REQUEST_CHANGES — the intended Cloudflare marker exception is broader than its stated contract and makes the dependency-cooldown gate fail open.

Blocking finding:

  • [P1] packages/dependency-cooldown/src/scan.ts:29: validate the marker before omitting bun.lockb. The rebuttal is correct that the Quantus Cloudflare repositories intentionally pair these files: the docs, explorer, and website checkouts each contain the same 189-byte plaintext # THIS IS JUST DUMMY FILE... marker beside the real bun.lock. However, omitBunLockbCoveredByTextLock does not recognize that marker; it suppresses every sibling bun.lockb, including a genuine binary lockfile. In a fresh Bun 1.1.33 reproduction, I began with both locks in sync, changed package.json, and regenerated only bun.lockb; Bun installed the changed dependency while bun.lock remained byte-for-byte unchanged, but the committed action printed Lockfiles: bun.lock, checked 0 introduced versions, and exited 0. A pull request can therefore replace the known marker with a real lockfile and move the installed graph without the cooldown gate checking or reporting it. That conflicts with this actions documented fail-closed rule that skipped dependencies are policy holes. Permit the actual marker by validating its content, reject an unknown or real paired bun.lockb, apply the rule consistently to head/base discovery, and add regressions covering both the real 189-byte marker and divergent non-marker lockfiles. Bun also documents migration as generating bun.lock and then deleting bun.lockb: https://bun.com/docs/pm/lockfile

Validation:

  • git diff --check f974da1e...173d6177 — passed.
  • npm ci — passed (one pre-existing low-severity audit item).
  • npm run typecheck — passed.
  • npm run build and committed-bundle diff — passed; dist is current.
  • npm test — passed, 80/80 tests.
  • Bun 1.1.33 divergent-lock reproduction against the committed bundle — action incorrectly passed with 0 checked versions.

No other blocking findings found.

@dewabisma
dewabisma requested a review from n13 August 27, 2026 13:44

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer model: GPT Sol

Verdict: APPROVE — the Cloudflare bun.lockb exception is now narrowly fail-closed and the prior divergent-lockfile blocker is resolved.

No blocking findings. The action ignores bun.lockb only when a sibling bun.lock exists and the binary path contains the exact 189-byte Quantus Cloudflare marker. Empty, altered, same-length impostor, and genuine binary lockfiles remain visible and are rejected by the existing Bun parser. The same marker validation is applied to both the checked-out head and the base revision, and the documentation and committed bundle match the source behavior.

Validation:

  • git diff --check f974da1e...b02325e0 — passed.
  • npm ci — passed (one pre-existing low-severity audit item).
  • npm run typecheck — passed.
  • npm run build plus committed-bundle diff check — passed; dist is current.
  • npm test — passed, 91/91 tests.
  • Live dependency-cooldown package CI — passed at b02325e0.

@dewabisma
dewabisma merged commit d513493 into main Aug 27, 2026
1 check passed
@dewabisma
dewabisma deleted the beast/fix-bug-detecting-bun-lockfile branch August 27, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants